Improve metadata tutorial #12931

cbrnr · 2024-10-31T10:54:48Z

I might have found some things that could be improved in the excellent metadata tutorial:

The text did not match what the code was doing in the stimulus-locked example (all correct epochs vs. slow correct epochs).
I am not sure what the "final sanity check" is supposed to show - the output only shows an empty table, but the text says "Bummer! It seems the very first two responses were recorded before the first stimulus appeared: the values in the stimulus column are None." I don't really see this in the output?
Maybe this is related to 2, but the ERN example (response-locked) shows a metadata table with 402 rows, whereas the stimulus-locked metadata has only 400 rows. This should not be the case, right?
The second-to-last plot shows two images side by side. First, I don't know how these were created (the code shows two independent plot commands). Second, the title of the right plot is cut off.
Mention actual authors at the top.

hoechenberger · 2024-10-31T15:18:00Z

Thanks @cbrnr, I might be able to take a look later today or tomorrow (I think I initially wrote a larger part of this very tutorial)

hoechenberger · 2024-11-03T14:37:11Z

I am not sure what the "final sanity check" is supposed to show - the output only shows an empty table, but the text says "Bummer! It seems the very first two responses were recorded before the first stimulus appeared: the values in the stimulus column are None." I don't really see this in the output?

I checked and it shows the correct output in the rendered documentation up until MNE-Python 1.6:

https://mne.tools/1.6/auto_tutorials/epochs/40_autogenerate_metadata.html#applying-the-knowledge-visualizing-the-ern-component

For newer versions, it generates an empty table, like you observed. I'll try to look into this.

hoechenberger · 2024-11-03T14:38:32Z

Maybe this is related to 2, but the ERN example (response-locked) shows a metadata table with 402 rows, whereas the stimulus-locked metadata has only 400 rows. This should not be the case, right?

Yes this is to be expected, there were 400 stimuli but 402 button presses. In our analysis, we wish to only consider the first button press following a stimulus.

This is directly related to your previous question / observation.

hoechenberger · 2024-11-03T14:52:36Z

Ok I found the problem.

MWE:

# %%
import mne

data_dir = mne.datasets.erp_core.data_path()
infile = data_dir / "ERP-CORE_Subject-001_Task-Flankers_eeg.fif"

raw = mne.io.read_raw(infile, preload=True)
raw.filter(l_freq=0.1, h_freq=40)
all_events, all_event_id = mne.events_from_annotations(raw)

metadata_tmin, metadata_tmax = -1.5, 0
row_events = ["response/left", "response/right"]
keep_last = ["stimulus", "response"]

metadata, events, event_id = mne.epochs.make_metadata(
    events=all_events,
    event_id=all_event_id,
    tmin=metadata_tmin,
    tmax=metadata_tmax,
    sfreq=raw.info["sfreq"],
    row_events=row_events,
    keep_last=keep_last,
)

# %%
metadata.loc[metadata["last_stimulus"] == "", :]

The cells with missing values are populated with empty strings. But there should be n/a in these instead.

@larsoner Any idea about this? It worked back with MNE-Python 1.6, and was broken in our rendered docs starting with 1.7. I'm not sure if it's anything we have changed, or if it's related to Pandas.

cbrnr · 2024-11-04T11:26:15Z

Also, didn't we agree to keep the actual authors in all tutorials and examples? Currently, this document just contains

# Authors: The MNE-Python contributors.

hoechenberger · 2024-11-04T13:39:02Z

I just tried with MNE 1.6 and I get the expected result. Switching to main while keeping all other installed packages the same yields the issue observed above. So it's likely a bug in MNE.

hoechenberger · 2024-11-04T13:49:15Z

@larsoner @cbrnr I think it's caused by this change from #12347

https://github.com/mne-tools/mne-python/pull/12347/files#diff-a6032a51226f9175d84fb340bab2a4238ddc24bf0c0f979bd1921f9d2f54c9fbR2993

cbrnr · 2024-11-04T14:12:11Z

Just to clarify the MWE, the output works for current versions (after the change), because it searches for metadata["last_stimulus"] == "", whereas MNE does metadata["last_stimulus"].isna().

larsoner · 2024-11-04T17:50:54Z

Also, didn't we agree to keep the actual authors in all tutorials and examples? Currently, this document just contains

I think we decided that people can opt-in to adding their names in tutorials and examples if they are so inclined. Sounds like the author(s) of that tutorial haven't done that yet!

doc/changes/devel/12931.bugfix.rst

cbrnr · 2024-11-05T07:22:56Z

tutorials/epochs/40_autogenerate_metadata.py

+ax[0].xaxis.set_visible(False)
+ax[1].xaxis.set_visible(False)
+
+fig


Can we not show the figure using plt.show()? Depending on where people execute this code, just typing fig might not actually show the figure.

I suppose @drammock or @larsoner would know an answer to this

But note that the tutorials are meant for interactive use and grouped into IPython cells, so …

Well, this wouldn't work in the standard Python interactive interpreter (unless interactive Matplotlib mode is enabled), it only works in a notebook. I was just wondering if plt.show() would be the canonical way to show a figure in both a regular interpreter and a notebook.

Whatever our expectation is, maybe worth clarifying that explicitly here? https://mne.tools/stable/auto_tutorials/index.html

I would use plt.show() in sphinx-gallery generated tutorials.

I'm happy with either as long as it works. can you make a suggestion or push a change?

Just to be clear, you are also relying on a notebook when you do

metadata

This will output an HTML table, so we cannot replace it with print(metadata) (and certainly we don't want to replace it with IPython.display.display()). So it's probably OK to assume we're in a notebook I guess (but we should clarify this on the tutorials and examples main pages)?

I would use plt.show() in sphinx-gallery generated tutorials.

I think this is what we've done elsewhere, or just remove the fig line

tutorials/epochs/40_autogenerate_metadata.py

mne/epochs.py

doc/changes/devel/12931.bugfix.rst

hoechenberger · 2024-11-05T13:04:07Z

@cbrnr You might be interested in this bug fix: 441c32e

hoechenberger · 2024-11-05T13:08:04Z

I think this is ready for a final review

I'll be withdrawing from this PR as I'm super busy with other stuff
For additional changes, please just push them yourselves 🙏

cbrnr · 2024-11-05T13:16:35Z

@cbrnr You might be interested in this bug fix: 441c32e

I'm not sure I understand what you mean...

hoechenberger · 2024-11-05T13:37:48Z

@cbrnr You might be interested in this bug fix: 441c32e

I'm not sure I understand what you mean...

The bug was introduced because the current behavior of the average parameter in the plotting functions is difficult to understand / use correctly, as you brought up in #12857

cbrnr · 2024-11-05T13:57:56Z

OK, this looks very good! One last (minor) gripe: we're using a line length of 88, which unfortunately is not rendered without a horizontal scrollbar (see the very last code block at the bottom of the document here). I opened #12945 for that so this can be merged!

larsoner · 2024-11-05T19:25:10Z

tutorials/epochs/40_autogenerate_metadata.py

+ax[0].xaxis.set_visible(False)
+ax[1].xaxis.set_visible(False)
+
+fig


I would use plt.show() in sphinx-gallery generated tutorials.

I think this is what we've done elsewhere, or just remove the fig line

tutorials/epochs/40_autogenerate_metadata.py

Co-authored-by: Clemens Brunner <[email protected]>

cbrnr · 2024-11-07T09:36:02Z

This is now finally ready for review and merge! Thanks @hoechenberger for addressing my comments!

larsoner · 2024-11-07T19:26:30Z

Failures are just from scikit-learn/scikit-learn#30237 so in it goes, thanks @cbrnr !

cbrnr requested review from drammock, agramfort and dengemann as code owners October 31, 2024 10:54

hoechenberger mentioned this pull request Nov 4, 2024

Regression in epochs metadata generation via change introduced in #12347 #12939

Closed

hoechenberger requested review from mscheltienne and larsoner as code owners November 4, 2024 21:34

hoechenberger marked this pull request as draft November 4, 2024 21:41

cbrnr commented Nov 5, 2024

View reviewed changes

mne/epochs.py Show resolved Hide resolved

hoechenberger mentioned this pull request Nov 5, 2024

Reconsider how missing values are represented in auto-generated epochs metadata #12943

Open

hoechenberger reviewed Nov 5, 2024

View reviewed changes

doc/changes/devel/12931.bugfix.rst Outdated Show resolved Hide resolved

hoechenberger marked this pull request as ready for review November 5, 2024 13:07

larsoner reviewed Nov 5, 2024

View reviewed changes

cbrnr and others added 3 commits November 7, 2024 10:34

Sync code with text

e3b8796

First set of improvements

81cc0c7

Plot 3 ERPs

c44de12

hoechenberger and others added 13 commits November 7, 2024 10:34

Rephrase

c5d7f5d

Address mne-tools#12939

5734d64

Add changelog

adc8431

[autofix.ci] apply automated fixes

235e8f6

Apply suggestions from code review

e5ed4e9

Co-authored-by: Clemens Brunner <[email protected]>

Update doc/changes/devel/12931.bugfix.rst

a7c2b19

Rephrasings

6986532

Fix averaging

f07d066

Structure and plot title

feef3c1

Smaller suptitle

3687006

Figure should already been shown

18fbe87

Remove fig (will be shown anyway)

206c098

Add Richard as author

e2cbabb

cbrnr force-pushed the fix-metadata-tutorial branch from fd8d376 to e2cbabb Compare November 7, 2024 09:35

hoechenberger enabled auto-merge (squash) November 7, 2024 13:21

larsoner disabled auto-merge November 7, 2024 19:26

larsoner merged commit 4608995 into mne-tools:main Nov 7, 2024
25 of 28 checks passed

cbrnr deleted the fix-metadata-tutorial branch November 11, 2024 10:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve metadata tutorial #12931

Improve metadata tutorial #12931

cbrnr commented Oct 31, 2024 •

edited

Loading

hoechenberger commented Oct 31, 2024

hoechenberger commented Nov 3, 2024

hoechenberger commented Nov 3, 2024 •

edited

Loading

hoechenberger commented Nov 3, 2024 •

edited

Loading

cbrnr commented Nov 4, 2024 •

edited

Loading

hoechenberger commented Nov 4, 2024

hoechenberger commented Nov 4, 2024

cbrnr commented Nov 4, 2024

larsoner commented Nov 4, 2024

cbrnr Nov 5, 2024

hoechenberger Nov 5, 2024

hoechenberger Nov 5, 2024

cbrnr Nov 5, 2024 •

edited

Loading

cbrnr Nov 5, 2024

mscheltienne Nov 5, 2024

hoechenberger Nov 5, 2024

cbrnr Nov 5, 2024 •

edited

Loading

larsoner Nov 5, 2024

hoechenberger commented Nov 5, 2024

hoechenberger commented Nov 5, 2024

cbrnr commented Nov 5, 2024

hoechenberger commented Nov 5, 2024 •

edited

Loading

cbrnr commented Nov 5, 2024 •

edited

Loading

larsoner Nov 5, 2024

cbrnr commented Nov 7, 2024

larsoner commented Nov 7, 2024

Improve metadata tutorial #12931

Improve metadata tutorial #12931

Conversation

cbrnr commented Oct 31, 2024 • edited Loading

hoechenberger commented Oct 31, 2024

hoechenberger commented Nov 3, 2024

hoechenberger commented Nov 3, 2024 • edited Loading

hoechenberger commented Nov 3, 2024 • edited Loading

cbrnr commented Nov 4, 2024 • edited Loading

hoechenberger commented Nov 4, 2024

hoechenberger commented Nov 4, 2024

cbrnr commented Nov 4, 2024

larsoner commented Nov 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cbrnr Nov 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cbrnr Nov 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hoechenberger commented Nov 5, 2024

hoechenberger commented Nov 5, 2024

cbrnr commented Nov 5, 2024

hoechenberger commented Nov 5, 2024 • edited Loading

cbrnr commented Nov 5, 2024 • edited Loading

Choose a reason for hiding this comment

cbrnr commented Nov 7, 2024

larsoner commented Nov 7, 2024

cbrnr commented Oct 31, 2024 •

edited

Loading

hoechenberger commented Nov 3, 2024 •

edited

Loading

hoechenberger commented Nov 3, 2024 •

edited

Loading

cbrnr commented Nov 4, 2024 •

edited

Loading

cbrnr Nov 5, 2024 •

edited

Loading

cbrnr Nov 5, 2024 •

edited

Loading

hoechenberger commented Nov 5, 2024 •

edited

Loading

cbrnr commented Nov 5, 2024 •

edited

Loading